Vendor MetadataKeyStore: debounced single-key sidecar writer#508
Merged
Conversation
Adds a small ``helpers/metadata_store.MetadataKeyStore`` modelled on Home Assistant's ``homeassistant.helpers.storage.Store`` for the "all state in RAM, write debounced to disk" pattern. Drops every HA dep we don't need (bus events, _StoreManager preload cache, version migration, _load_future reentrancy guard); keeps the parts that earn complexity: * Debounce + extend semantics matching HA bit-for-bit — calls during an open delay window update the latest deadline; the timer reschedules itself when it wakes early. * Lock-protected single-flight writes so a stop()-triggered final flush can't race a delayed-handler-driven write. * data_func captured at every schedule, invoked at flush time, so the persisted snapshot reflects the controller's latest in-RAM state instead of stale data from when the first call queued the handle. The store doesn't reach into our shared metadata sidecar directly; callers inject ``load_sync`` / ``write_sync`` so the heavy lifting stays in ``controllers.config`` (which owns the lock + atomic write). No consumers wired up here — landing the helper first so the upcoming receiver / offloader pairings RAM-only refactors can build on a reviewed primitive. Adds ``hass`` to the codespell ignore list (the docstring references HA's storage module).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #508 +/- ##
=======================================
Coverage 99.13% 99.14%
=======================================
Files 67 68 +1
Lines 8007 8070 +63
=======================================
+ Hits 7938 8001 +63
Misses 69 69
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR vendors a new MetadataKeyStore helper that implements the “keep state in RAM, debounce writes to disk” pattern for a single sub-key of the shared metadata sidecar, matching Home Assistant’s debounce/extend semantics while delegating actual I/O (and locking/atomicity) to injected callbacks.
Changes:
- Added
esphome_device_builder.helpers.metadata_store.MetadataKeyStoreimplementing debounced, single-flight async writes via executor +asyncio.Lock. - Added a comprehensive async test suite covering debounce collapsing, deadline extension, flush semantics, and write-failure swallowing.
- Updated
codespellignore list to includehass(used in docstrings referencing Home Assistant).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
esphome_device_builder/helpers/metadata_store.py |
Introduces the debounced single-key metadata store helper with HA-matching scheduling semantics. |
tests/test_helpers_metadata_store.py |
Adds unit tests validating scheduling/flush behavior, inflight-write coordination, and error logging. |
pyproject.toml |
Extends codespell ignore list to allow hass in documentation strings. |
Promotes the shutdown-flush wiring from "remember to call async_save_now() in stop()" to a structural contract: the constructor now requires a shutdown_register callback, invoked exactly once with self.async_save_now during __init__. The caller's lifecycle layer (typically DeviceBuilder.shutdown_callbacks) holds the resulting list and awaits every callback at graceful stop. A store can no longer be instantiated without telling someone who will flush it; tests that genuinely don't care can pass ``lambda _cb: None`` to opt out, but production paths must wire a real registry. The hard-kill caveat (SIGKILL / process crash) is unchanged from HA's Store and documented inline. Adds ShutdownCallback / ShutdownRegister PEP 695 type aliases so call sites don't have to spell the nested Callable inline, and a test exercising the cross-store lifecycle drain (multiple stores sharing one registry, one walk flushes them all).
Two real catches:
* Class docstring referenced ``*_T*`` but the PEP-695 type
parameter is ``T``; renamed the docstring callout so a future
reader doesn't trip over the mismatch.
* Error log line ``"Error writing metadata key"`` carried no
identifying context — a production failure trace couldn't tell
*which* sub-key broke. Adds an optional ``name`` constructor arg
(typically the sidecar sub-key, e.g.
``"_offloader_remote_build"``) that surfaces in both the asyncio
task name and the error log line, plus the ``config_dir`` for
good measure. Defaults to ``"<unnamed:{config_dir}>"`` so a
caller that omits the arg still gets *something* identifying.
New tests pin the diagnostic shape (name + config_dir present in
the log line) and the default-name fallback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this implement/fix?
Vendors a small
helpers/metadata_store.MetadataKeyStoremodelled onHome Assistant's
homeassistant.helpers.storage.Storefor the"all state in RAM, write debounced to disk" pattern. The intent is
to land the helper first so the upcoming receiver / offloader
pairings RAM-only refactors can build on a reviewed primitive (and
so a future audit can flip
_devices,_labels,_prefs, etc. ontoit incrementally). No consumers wired up in this PR;
MetadataKeyStoreimports cleanly and ships at 100% coverage.
The store doesn't reach into our shared metadata sidecar directly —
callers inject
load_sync/write_syncso the heavy liftingstays in
controllers.config(which already owns the lock + atomicwrite).
Drops every HA dep we don't need: bus events
(
EVENT_HOMEASSISTANT_FINAL_WRITE),_StoreManagerpreload cache,version migration,
_load_futurereentrancy guard. Keeps the partsthat earn complexity:
during an open delay window update the latest deadline; the timer
reschedules itself when it wakes early.
stop()-triggeredfinal flush can't race a delayed-handler-driven write.
data_funccaptured at every schedule, invoked at flush time,so the persisted snapshot reflects the controller's latest
in-RAM state instead of stale data from when the first call
queued the handle.
Adds
hassto the codespell ignore list (the docstring referencesHA's storage module by path).
Related issue or feature (if applicable):
PR follows once this lands.
Types of changes
bugfixnew-featureenhancementbreaking-changerefactordocsmaintenancecidependenciesFrontend coordination
Checklist
ruff,codespell, yaml/json/python checks).tests/where applicable.components.jsonhas not been hand-edited (regenerate viascript/sync_components.pyif a sync is needed).docs/ARCHITECTURE.mdand/ordocs/API.md.